-
Notifications
You must be signed in to change notification settings - Fork 38
Normalize SDK errors for clients #980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…auth/rate-limit/timeout/service-unavailable/bad-request\n- Map provider/litellm errors to SDK types in LLM._map_exception\n- Agent: when no condenser, rethrow mapped typed errors; still trigger condensation when available\n\nCo-authored-by: openhands <[email protected]>
This comment has been minimized.
This comment has been minimized.
…ts\n\n- Address remaining E501 by shortening comments\n- No functional changes\n\nCo-authored-by: openhands <[email protected]>
Co-authored-by: openhands <[email protected]>
…ceeded in classifier Co-authored-by: openhands <[email protected]>
…h mapping check - Default message now reflects current behavior: suggest enabling a condenser or shortening inputs - Update unit test to assert new message - mapping.py: simplify to only call looks_like_auth_error() as it already checks exception types Co-authored-by: openhands <[email protected]>
…ssing Co-authored-by: openhands <[email protected]>
…vailable/InternalServer errors to LLMServiceUnavailableError - Avoid mapping APIConnectionError to LLMServiceUnavailableError to keep backward-compatibility and satisfy retry unit test expectations Co-authored-by: openhands <[email protected]>
Coverage Report •
|
||||||||||||||||||||||||||||||||||||||||||||||||||
…ry test to expect SDK typed error after max retries - Provide consistent SDK exceptions for connectivity/service availability cases - Keep retry mechanism unchanged; tests now validate new mapped exception type Co-authored-by: openhands <[email protected]>
Remove redundant test that asserted ContextWindowExceededError → LLMContextWindowExceedError mapping, which is already covered by classifier tests and typed exception message tests. Keeps mapping suite focused on auth vs generic BadRequest and passthrough behavior. Co-authored-by: openhands <[email protected]>
…t exception cause - Promote import to top-level per review comment - Verify __cause__ is litellm.exceptions.APIConnectionError to ensure PR preserves provider exception chaining Co-authored-by: openhands <[email protected]>
…r errors - Keep condenser+context-window handling in Agent - Otherwise re-raise since LLM.completion/responses already map to SDK-typed errors Co-authored-by: openhands <[email protected]>
- Import is_context_window_exceeded and map_provider_exception without aliasing - Update wrapper methods to call explicit names Co-authored-by: openhands <[email protected]>
…tly at call sites Co-authored-by: openhands <[email protected]>
xingyaoww
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just one comment
| return reconciled | ||
|
|
||
| @staticmethod | ||
| def is_context_window_exceeded_exception(exception: Exception) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we replace all occurrences of this in favor of the new API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! The agent actually only needs to catch the SDK exception.
…ext_window_exceeded_exception wrapper - Trigger condensation via except LLMContextWindowExceedError when condenser handles requests - Simplify LLM by removing back-compat wrapper/alias Co-authored-by: openhands <[email protected]> # Conflicts: # openhands-sdk/openhands/sdk/agent/agent.py # openhands-sdk/openhands/sdk/llm/llm.py
…-authored-by: openhands <[email protected]>
…rors\n\nCo-authored-by: openhands <[email protected]>
xingyaoww
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Co-authored-by: openhands <[email protected]>
|
@OpenHands Read the initial description. It says in follow ups:
Lets document, but not in the README, instead clone yourself the docs repo from the same org, understand the structure and find the best place to add docs on SDK Exceptions for client developers to know and handle as they want. Then add it. Open a PR to docs. |
|
I'm on it! enyst can track my progress at all-hands.dev |
- New guide: sdk/guides/error-handling.mdx - Navigation: add under SDK > Guides > LLM Features Refs OpenHands/software-agent-sdk#980 Co-authored-by: openhands <[email protected]>
- Show try/except around LLM.responses() Refs OpenHands/software-agent-sdk#980 Co-authored-by: openhands <[email protected]>
|
Summary of changes since last update I completed the documentation work for SDK Error Handling & Exceptions and opened a PR to the docs repository. Key updates:
Checklist
Diff summary (concise)
PR link |
- Rename file to align with other LLM features - Update navigation to point at new path Refs OpenHands/software-agent-sdk#980 Co-authored-by: openhands <[email protected]>
- Lead with Agent/Conversation examples and bubbling behavior - Move LLM examples (completion/responses) into secondary section Refs OpenHands/software-agent-sdk#980 Co-authored-by: openhands <[email protected]>
…uide - Content is redundant and covered elsewhere Refs OpenHands/software-agent-sdk#980 Co-authored-by: openhands <[email protected]>
* SDK: Document error handling & typed exceptions - New guide: sdk/guides/error-handling.mdx - Navigation: add under SDK > Guides > LLM Features Refs OpenHands/software-agent-sdk#980 Co-authored-by: openhands <[email protected]>
Summary
Fix #979
This PR introduces a provider-agnostic error surface for the SDK and improves the context-window exceeded path when no condenser is configured.
What’s in this PR
Motivation
Currently, clients receive raw LiteLLM/OpenAI exceptions (BadRequestError, OpenAIError, etc.) for common cases like context window exceeded or invalid API key, which are not stable across providers. Normalizing these enables client apps (e.g., openhands-cli, integrations) to handle errors predictably.
Follow-ups suggested in issue
Refs
Co-authored-by: openhands [email protected]
@enyst can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
golang:1.21-bookwormeclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22Pull (multi-arch manifest)
Run
All tags pushed for this build
The
72d8f2ctag is a multi-arch manifest (amd64/arm64); your client pulls the right arch automatically.